Skip to content

Fix for content-type header with charset#110

Closed
nithin-murali-arch wants to merge 1 commit intozerodha:masterfrom
nithin-murali-arch:patch-1
Closed

Fix for content-type header with charset#110
nithin-murali-arch wants to merge 1 commit intozerodha:masterfrom
nithin-murali-arch:patch-1

Conversation

@nithin-murali-arch
Copy link

Issue with content-type header with [email protected] where the content-type header is application/json;charset=utf8 which breaks responses.

Issue with content-type header with [email protected] where the content-type header is application/json;charset=utf8 which breaks responses.
@vishalmanes109
Copy link

The fix is working
Thanks for the PR

@ranjanrak
Copy link
Member

I've implemented this fix for all content-type headers (including text/csv with contentType?.includes('text/csv') - this was missing in your PR) in v5.1.0. Thanks for your PR.

@ranjanrak ranjanrak closed this Jul 23, 2025
@nithin-murali-arch
Copy link
Author

@ranjanrak . Don't open source stuff if you're not able to respond on time to fix issues. Look at the diff, where is text/csv there? - This is for "this was missing in your PR"

@ranjanrak
Copy link
Member

Look at the diff, where is text/csv there?

You can refer to the implementation in lib/connect.ts at line 264.

I have clearly mentioned in my earlier reply, if you're following the pattern(.includes) instead of strict equality(===), it should be applied consistently everywhere for better maintainability.

@nithin-murali-arch
Copy link
Author

I'm not here to debate on the maintainability or my PR. Im here to talk about the issue I faced, and your unnecessary comment on "what I missed". I wrote a PR for the issue I faced, its not my job to search your repo for where else it needs to be fixed. You're a for profit venture, and this project is for profit. You're not doing this out of good will, remember that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants